Update logout command to revoke token server-side#1733
Update logout command to revoke token server-side#1733amritghimire wants to merge 1 commit intomainfrom
Conversation
Context -------- The datachain auth logout command currently only deletes the token from local config. The Studio backend now exposes POST /api/token-logout for self-revocation. The logout command should call this endpoint before clearing the local token, so the token is invalidated both locally and server-side. This changes calls studio endpoint on logout
Deploying datachain with
|
| Latest commit: |
9eb6186
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://04ef45ec.datachain-2g6.pages.dev |
| Branch Preview URL: | https://amrit-logout-token.datachain-2g6.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR updates the DataChain Studio auth logout CLI behavior to revoke the active token server-side via the new Studio endpoint before removing it from local configuration, ensuring logout invalidates credentials both locally and remotely.
Changes:
- Call
POST /api/token-logoutduring Studio logout prior to deleting the local token. - Add stderr warnings for “already revoked/invalid” tokens and for unexpected/unreachable Studio responses.
- Extend CLI test coverage for successful revocation, 401 “already revoked”, and custom Studio URLs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/datachain/studio.py |
Implements server-side token revocation during logout and adds warning handling for error scenarios. |
tests/test_cli_studio.py |
Updates logout tests to assert the revoke endpoint is called and adds new logout test scenarios (401 + custom URL). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| studio_url = ( | ||
| conf.get("studio", {}).get("url") | ||
| or get_studio_env_variable("URL") | ||
| or STUDIO_URL | ||
| ) |
There was a problem hiding this comment.
logout() resolves studio_url from config before checking DATACHAIN_STUDIO_URL (get_studio_env_variable("URL")). Elsewhere, Studio URL resolution prioritizes the env var (e.g., StudioClient.url and login()), so when DATACHAIN_STUDIO_URL is set this can revoke the token against a different host than the rest of the CLI uses. Consider aligning the precedence (env var first) or reusing a shared URL-resolution helper to keep behavior consistent across commands.
| def test_studio_logout(): | ||
| with Config(ConfigLevel.GLOBAL).edit() as conf: | ||
| conf["studio"] = {"token": "isat_access_token"} | ||
| conf["studio"] = {"token": "isat_access_token", "url": STUDIO_URL} | ||
|
|
||
| with requests_mock.mock() as m: | ||
| m.post( | ||
| f"{STUDIO_URL}/api/token-logout", | ||
| json={"detail": "Token revoked successfully"}, | ||
| ) | ||
| assert main(["auth", "logout"]) == 0 |
There was a problem hiding this comment.
test_studio_logout() now sets studio.url in config, so it no longer exercises the fallback path where only a token is present (legacy configs) and logout() should default to DATACHAIN_STUDIO_URL/STUDIO_URL. Adding a test case for token-only config (and optionally env-var URL override) would help prevent regressions in the new URL-selection logic.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| file=sys.stderr, | ||
| ) | ||
| except requests.RequestException: | ||
| print( |
There was a problem hiding this comment.
let's be strict then - don't proceed with logout at all
There was a problem hiding this comment.
The problem with that is in case user deletes the token from UI, then tries to clear it from cli too, it will raise error and user can't remove the token from the config. It will be a pain point for user and blocks further process.
There was a problem hiding this comment.
let's handle not found properly then (assuming it was deleted already)
There was a problem hiding this comment.
Consider cleaning up tokens from Studio before deleting them locally. Abort the logout process with an error if the Studio logout request fails, giving users the responsibility to re-try.
Studio should be able to emit distinct responses for "this token is well-formed but no longer valid" (logout can continue and local token can be deleted) versus anything else (server error, network error, ...)
| ) | ||
| elif not response.ok: | ||
| print( | ||
| f"Warning: Unexpected response from Studio " |
There was a problem hiding this comment.
same here - don't proceed then
There was a problem hiding this comment.
Same reasoning above.
|
check also tests, we need to hit way more coverage |
Context
The datachain auth logout command currently only deletes the token from local config. The Studio backend now exposes POST /api/token-logout for self-revocation. The logout command should call this endpoint before clearing the local token, so the token is invalidated both locally and server-side.
This changes calls studio endpoint on logout